fix: resolve submodules from the current checkout's .gitmodules - #2767
fix: resolve submodules from the current checkout's .gitmodules#2767TheGreatApollyon wants to merge 2 commits into
Conversation
When a template moves one of its submodules to a new repository, `copier update` fails on the newer checkout with `fatal: remote error: upload-pack: not our ref <commit>`. Worktrees created from the cached mirror share the mirror's config, so `git submodule update --init` in an earlier checkout registers `submodule.<name>.url` entries pointing at that checkout's submodule URLs. Those registrations override the .gitmodules of later checkouts, which then try to fetch the pinned commit from the stale URL. Drop any `submodule.<name>.url` registrations from the mirror's config before updating submodules, so every checkout resolves them from its own .gitmodules.
There was a problem hiding this comment.
Pull request overview
This PR fixes a copier update failure mode when a template submodule’s URL changes between revisions by ensuring each cached worktree resolves submodules from its own .gitmodules rather than stale submodule.<name>.url entries persisted in the shared mirror config.
Changes:
- In the cached-mirror worktree flow, removes any
submodule.<name>.urlentries from the mirror’sconfigbefore runninggit submodule update. - Adds a regression test that creates a template whose submodule URL changes between two tags and verifies both revisions check out the correct submodule content.
- Adds test environment configuration to allow local (
file:///path) submodules under modern Git defaults.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
copier/_vcs.py |
Cleans stale submodule.*.url entries from the shared mirror config before submodule checkout in cached worktrees. |
tests/test_vcs.py |
Adds a regression test for submodule URL moves across revisions, plus Git env setup to allow local submodules in tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @pytest.fixture(scope="session", autouse=True) | ||
| def allow_file_submodules() -> None: | ||
| """Allow the fixture repos below to be used as submodules. | ||
|
|
||
| Since Git 2.38.1 the file protocol is blocked for submodules by default | ||
| (see GHSA-3wp6-j8xr-qw85), so local submodules require this setting. Set | ||
| it via the environment so it also applies to Copier's own subprocesses. | ||
| """ | ||
| local.env["GIT_CONFIG_COUNT"] = "1" | ||
| local.env["GIT_CONFIG_KEY_0"] = "protocol.file.allow" | ||
| local.env["GIT_CONFIG_VALUE_0"] = "always" |
There was a problem hiding this comment.
Fixed — changed to scope="module" with with local.env(...): yield so the environment variables are automatically restored after each test module. Good catch, thanks!
sisp
left a comment
sisp
left a comment
There was a problem hiding this comment.
Thanks for discovering this bug and submitting a PR that fixes it, @TheGreatApollyon! 🙇
I've left two inline suggestions.
| @pytest.fixture(scope="module", autouse=True) | ||
| def allow_file_submodules() -> None: | ||
| """Allow the fixture repos below to be used as submodules. | ||
|
|
||
| Since Git 2.38.1 the file protocol is blocked for submodules by default | ||
| (see GHSA-3wp6-j8xr-qw85), so local submodules require this setting. Set | ||
| it via the environment so it also applies to Copier's own subprocesses. | ||
| """ | ||
| with local.env( | ||
| GIT_CONFIG_COUNT="1", | ||
| GIT_CONFIG_KEY_0="protocol.file.allow", | ||
| GIT_CONFIG_VALUE_0="always", | ||
| ): | ||
| yield |
There was a problem hiding this comment.
How about setting
git = git["-c", "protocol.file.allow=always"]
at the beginning of test_remote_clone_submodule_with_moved_url instead of using this autouse fixture? This would avoid enabling the file protocol for the entire module and setting Git config environment variables (which would need incrementing GIT_CONFIG_COUNT instead of overwriting it and using the new index for GIT_CONFIG_{KEY,VALUE}_<N> even)?
| # Worktrees share the mirror's config, so `git submodule update | ||
| # --init` from an earlier checkout may have registered | ||
| # `submodule.<name>.url` entries pointing at that checkout's | ||
| # submodule URLs, silently overriding the current `.gitmodules` | ||
| # (e.g. after a submodule moved to a new repository). Drop any | ||
| # registrations so each checkout resolves its submodules from its | ||
| # own `.gitmodules`. | ||
| for key in git( | ||
| "config", | ||
| "--file", | ||
| str(mirror / "config"), | ||
| "--get-regexp", | ||
| r"^submodule\..+\.url$", | ||
| retcode=None, | ||
| ).splitlines(): | ||
| git( | ||
| "config", | ||
| "--file", | ||
| str(mirror / "config"), | ||
| "--unset-all", | ||
| key.split()[0], | ||
| ) |
There was a problem hiding this comment.
I think we can simplify and robustify this using git submodule sync:
| # Worktrees share the mirror's config, so `git submodule update | |
| # --init` from an earlier checkout may have registered | |
| # `submodule.<name>.url` entries pointing at that checkout's | |
| # submodule URLs, silently overriding the current `.gitmodules` | |
| # (e.g. after a submodule moved to a new repository). Drop any | |
| # registrations so each checkout resolves its submodules from its | |
| # own `.gitmodules`. | |
| for key in git( | |
| "config", | |
| "--file", | |
| str(mirror / "config"), | |
| "--get-regexp", | |
| r"^submodule\..+\.url$", | |
| retcode=None, | |
| ).splitlines(): | |
| git( | |
| "config", | |
| "--file", | |
| str(mirror / "config"), | |
| "--unset-all", | |
| key.split()[0], | |
| ) | |
| git("submodule", "sync", "--recursive") |
WDYT?
Summary
When a template's submodule moves to a new repository between versions,
copier updatefails while checking out the newer version:Root cause
Worktrees created from the cached mirror share the mirror's config.
git submodule update --initin an earlier checkout registerssubmodule.<name>.urlentries there, pointing at that checkout's submodule URLs. Those registrations silently override the.gitmodulesof later checkouts (the config takes precedence over.gitmodules), so the newer checkout tries to fetch its pinned commit from the old, stale location and fails.Fix
Before running
git submodule updateon a worktree, drop anysubmodule.<name>.urlregistrations from the mirror's config. Every checkout then resolves its submodules from its own.gitmodules.This also fixes updates for users who already have a polluted mirror cache, since the cleanup runs on every checkout.
Test
Added a regression test that clones a template whose submodule moved (URL changed between two tags), asserting that both revisions check out with the correct submodule content. Verified it fails on the previous code with the exact error above.
Fixes #2766